Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nixos/zfs: order pool sync services before final.target #347329

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bjornfor
Copy link
Contributor

@bjornfor bjornfor commented Oct 8, 2024

Things done

If we don't do this, I think systemd can kill the service(s) before they
finish.

Disclaimer: Not tested, but I noticed in another service (NUT) that this
configuration was needed to avoid killed service. I found the ZFS module by
searching for services using git grep wantedBy.*shutdown.

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

If we don't do this, I think systemd can kill the service(s) before they
finish.
@Zocker1999NET
Copy link
Contributor

This is a great suggestion.

I think it would be really helpful if one could link to some docs or some comment explaining this possible behavior of systemd a little bit further. Having such link in the PR (or IMO in the commit message) makes it much easier to later understand why that config was added (esp. in case that behavior of systemd needs to change in the future).

You mentioned another "NUT" service where this setting is already required. (I assume "NUT" stands for "Network UPS Tools".) Here in nixpkgs (at current master) I only found one service which also has Before=final.target set, which is apcupsd, did you mean that one?

before = [ "final.target" ];

The explaining link there from the OpenSUSE forum sadly seems broken (& archive.org is down too at time of writing …). I assume this post was migrated to this location, but that does also not explain why/if final.target is required.
The commit 95e2006 introducing that line is also missing further explanation.

@bjornfor
Copy link
Contributor Author

Re: NUT, I was thinking of #344940.

I think it would be really helpful if one could link to some docs or some comment explaining this possible behavior of systemd a little bit further. Having such link in the PR (or IMO in the commit message) makes it much easier to later understand why that config was added (esp. in case that behavior of systemd needs to change in the future).

Good idea, but the only place I remember that talk about this is man systemd.special, and it doesn't really go into enough details (or I haven't found them).

I'm also the author of the apcupsd.nix module, but that was like 10 years ago. I guess I did the same trial and error testing and came to the conclusion that before = [ "final.target" ] was a good thing to have back then too :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants